fixed compiler warnings #4741
fixed compiler warnings #4741
Conversation
fixed warning: vars may be used uninitialized in this function
fixed compiler warning: var is unused
fix_warning
WalkthroughThe changes update variable initialization and conditional compilation in two source files. In Changes
Possibly related PRs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
wled00/FX.cpp (1)
7862-7863: Prefer fixed-width types and consistent sizing for angle variables
Usingunsignedforangleincrementis ambiguous; preferuint16_t(or another exact-width type) for consistency withangle.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wled00/FX.cpp(1 hunks)wled00/cfg.cpp(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: wled_build / Build Environments (esp32c3dev)
- GitHub Check: wled_build / Build Environments (esp32_wrover)
- GitHub Check: wled_build / Build Environments (esp32dev_V4)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (esp32dev)
- GitHub Check: wled_build / Build Environments (lolin_s2_mini)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp32_eth)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
- GitHub Check: wled_build / Build Environments (esp01_1m_full)
- GitHub Check: wled_build / Build Environments (nodemcuv2_160)
- GitHub Check: wled_build / Build Environments (esp8266_2m)
- GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
- GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
- GitHub Check: wled_build / Build Environments (nodemcuv2)
- GitHub Check: wled_build / Build Environments (esp8266_2m_160)
- GitHub Check: wled_build / Test cdata.js
🔇 Additional comments (4)
wled00/cfg.cpp (3)
778-783: LGTM - Appropriate conditional compilation for EEPROM features.The conditional compilation correctly guards EEPROM-related function calls when EEPROM support is disabled, preventing compilation of unused code.
1275-1277: Good optimization - conditional interfaces object creation.The conditional compilation correctly avoids creating the
interfacesJsonObject when both MQTT and HueSync features are disabled, reducing memory usage and processing overhead.
1319-1321: Consistent conditional compilation for serialization.The same conditional compilation pattern is correctly applied to the serialization path, maintaining consistency with the deserialization changes.
wled00/FX.cpp (1)
7859-7859: Good suppression of unused-variable warnings and defined initialization
Initializingspeedto a safe default and marking it[[maybe_unused]]cleanly resolves the compiler warning.
| #ifdef WLED_ADD_EEPROM_SUPPORT | ||
| success = readObjectFromFile(s_cfg_json, nullptr, pDoc); | ||
| #endif |
There was a problem hiding this comment.
Critical logic error: cfg.json reading should not depend on EEPROM support.
The readObjectFromFile call reads from the filesystem (/cfg.json), not from EEPROM. Making this conditional on WLED_ADD_EEPROM_SUPPORT means the configuration file won't be read when EEPROM support is disabled, causing deserializeConfig() to operate on an empty JsonObject.
This breaks the core configuration loading functionality when EEPROM support is compiled out.
- #ifdef WLED_ADD_EEPROM_SUPPORT
success = readObjectFromFile(s_cfg_json, nullptr, pDoc);
- #endif🤖 Prompt for AI Agents
In wled00/cfg.cpp around lines 789 to 791, remove the #ifdef
WLED_ADD_EEPROM_SUPPORT conditional around the readObjectFromFile call so that
the configuration file is always read regardless of EEPROM support. This ensures
deserializeConfig() receives the correct JsonObject from the filesystem and
prevents loading failures when EEPROM support is disabled.
| // variables for circular explosions | ||
| [[maybe_unused]] int32_t speed, currentspeed, speedvariation, percircle; | ||
| [[maybe_unused]] int32_t speed = 2; // just init to min value | ||
| [[maybe_unused]] int32_t currentspeed, percircle; |
There was a problem hiding this comment.
Initialize or remove uninitialized variables currentspeed and percircle
These variables are declared uninitialized and marked [[maybe_unused]], which risks undefined behavior if they later become used. Either initialize them explicitly (e.g., = 0) or remove them if truly unused.
🤖 Prompt for AI Agents
In wled00/FX.cpp at line 7860, the variables currentspeed and percircle are
declared but uninitialized and marked [[maybe_unused]], which can lead to
undefined behavior if used later. To fix this, either initialize both variables
explicitly to zero or another appropriate default value, or remove their
declarations entirely if they are not used anywhere in the code.
|
thanks for the input. |
I don't think that the code will be bigger just because some var is initialized and the are better managed compile masks |
try it. |
|
I tryed, and the compile output is even smaller. I am comparing the current PR with the main branch: "a", "b" and "c" are my custom builds. Initializing variables is good practice to avoid warnings about uninitialized use, which can lead to undefined behavior. What do you think about it? |
I do see the sum of your commits saves on flash, did not investigate why yet. But the changes you made to FX.cpp which is adding technically unneeded inits uses a few bytes of extra flash. |
Dear @DedeHai , technically yes, initializing variables requires flash but, in this case, we are talking about 6 or 8 bytes that compared to 4MB of flash is a very small increase. Anyway I don't want to convince anyone about this good coding practice.😌 Anyway if we talk about flash reduction, this PR saves about 600 bytes, feel free to check it, please! Thanks for collaborating in this project😁 |
|
the reason this PR saves flash: it removes config and breaks everything. |
2 participants
fixed compiler warnings for uninitialized vars
fixed compiler warnings for unused variables
Summary by CodeRabbit
Bug Fixes
Refactor